Add komi-learn update self-update command#8
Merged
Conversation
Users had no in-CLI way to upgrade after a new PyPI release. `update` checks PyPI for a newer version and upgrades in place via the right package manager: - pipx when running from a pipx-managed venv (`pipx upgrade`) - otherwise pip into the *running* interpreter (the one the hooks import), mirroring model_install.py - if neither can be determined safely, print the command instead of guessing and risking a broken environment `--check` reports availability without upgrading; `--yes` skips the confirm. Network failures are non-fatal. After upgrading, the new version is read from a fresh subprocess (importlib.metadata is cached in-process). Also fixes a stale `__version__` (hardcoded 0.1.0 while pyproject was 0.3.0): it now derives from installed distribution metadata, so doctor and update both report the truth and the two never drift again. Tests: tests/test_update.py (25) — version compare incl. 0.10>0.9, PyPI lookup + offline/malformed, pip/pipx detection, undetectable fallback, upgrade success/failure, and all CLI routing branches. Full suite 240 passed, 1 skip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adversarial review (security + AI-eng + architect) found real issues; fixed all: Security: - check_latest now refuses HTTP redirects (no-redirect opener) and verifies the final URL is https -- the hardcoded literal only guaranteed the first hop, so a MITM could downgrade to http and feed a spoofed "newer" version. Bound the response read at 5MB (DoS guard). - pipx upgrade resolves pipx to an ABSOLUTE path via shutil.which (was unqualified "pipx" off PATH -- a planted binary could run during update); refuses if not found. Tightened _is_pipx so a bare PIPX_HOME env var no longer flips a pip install into the pipx branch -- the interpreter must actually live under it. Correctness (AI-eng): - Rewrote version comparison as a self-contained PEP 440-lite comparator and DROPPED the optional packaging dependency. The old fallback disagreed with packaging on rc/post/dev/epoch and on 1.0 vs 1.0.0, so the answer varied by environment -- a real downgrade/no-op-as-upgrade bug for our likely 0.4.0rc releases. Now one code path: pads tuples (1.0==1.0.0), strips leading v, orders dev < pre(a<b<rc) < final < post, honors epoch. - cmd_update no longer claims "upgraded to X" when the post-upgrade re-check can't confirm the version (pip exiting 0 isn't proof); says "couldn't confirm" instead. Architecture: - Deleted dead UpdateResult dataclass (exported, never constructed). - Single source of truth for the version: pyproject reads komi.__version__ via setuptools dynamic version, killing the _FALLBACK_VERSION literal drift. Build + twine check verified (wheel/sdist resolve to 0.3.0). Tests: fixed the tautological --check assert (both sides were identical and the string matched a different branch); added is_newer cases for rc/post/dev/epoch + 1.0/1.0.0 + leading-v + a total-ordering chain; added redirect-refusal, non-https-final-url, oversized-body, pipx-abs-path, pipx-not-found, and bare-PIPX_HOME-insufficient tests; added a dynamic-version drift guard. test_update.py 25 -> 51. Full suite 266 passed, 1 skip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
Three-persona review applied (security + AI engineer + architect)All three returned SHIP-WITH-FIXES. Every flagged NOW-fix is in Security
Correctness
Architecture
Tests: fixed the tautological Deferred (not in this PR, intentionally): lazy version import on the hook hot path, extracting a shared |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds
komi-learn update— check PyPI for a newer release and upgrade in place.Why
There was no in-CLI way to upgrade after a new PyPI release (0.1.0 -> 0.2.0 -> 0.3.0 were all manual
pip install -U). Users expectupdateto exist.How it upgrades the right environment
The hard part isn't the PyPI check — it's not breaking the user's environment.
plan_upgrade()detects the install method:PIPX_*env set) ->pipx upgrade komi-learn[sys.executable, -m, pip, install, --upgrade, komi-learn]— upgrades the running interpreter, the one the hooks import (mirrorsmodel_install.py)After upgrade, the new version is read from a fresh subprocess because
importlib.metadatais cached for the life of the running process.Also fixes
komi/__init__.pyhad a stale__version__ = "0.1.0"whilepyprojectwas at0.3.0(doctor printed the wrong version). It now derives from installed distribution metadata, sodoctorandupdateboth report the truth and the two can't drift again. The literal is kept only as a source-tree fallback.Tests
tests/test_update.py— 25 tests: version compare (incl.0.10.0 > 0.9.0, which a string compare gets wrong), PyPI lookup + offline/malformed payloads, pip/pipx detection, undetectable fallback, upgrade success/failure/missing-binary, and every CLI routing branch. Network and subprocess are fully mocked.Full suite: 240 passed, 1 skipped. Smoke-tested live:
update --checkagainst PyPI correctly reports being on the latest version.🤖 Generated with Claude Code